ranger: add a more common method to calc range. #3489
Conversation
| @@ -22,6 +22,11 @@ import ( | |||
| "github.com/pingcap/tidb/sessionctx/variable" | |||
| ) | |||
|
|
|||
| // Range is the interface of the three type of range. | |||
There was a problem hiding this comment.
- I think this file can be moved to package util/ranger
- I think this interface can implement convert2***Range
There was a problem hiding this comment.
1st comment make the pr a little large, move this to a new pr.
2nd done.
expression/column.go
Outdated
| return nil | ||
| } | ||
|
|
||
| // indexCol2Col finds the correspond column of the IndexColumn in a column slice. |
expression/column.go
Outdated
| @@ -245,3 +245,43 @@ func Column2Exprs(cols []*Column) []Expression { | |||
| } | |||
| return result | |||
| } | |||
|
|
|||
| // ColInfo2Col finds the correspond column of the ColumnInfo in a column slice. | |||
plan/new_physical_plan_builder.go
Outdated
| ts.Ranges, err = ranger.BuildTableRange(ts.AccessCondition, sc) | ||
| if err != nil { | ||
| return nil, errors.Trace(err) | ||
| if pkColumn != nil { |
There was a problem hiding this comment.
the GetPkColInfo may return nil, it's not a good code style. Here we can check pk is handle and make sure pk column is not nil.
| } | ||
|
|
||
| // BuildRange is a method which can calculate IntColumnRange, ColumnRange, IndexRange. | ||
| func BuildRange(sc *variable.StatementContext, conds []expression.Expression, rangeType int, cols []*expression.Column, lengths []int) (retRanges []types.Range, |
There was a problem hiding this comment.
I think it's no need to pass every column's length. It's enough to know whether the last column has specified length. e.g. if the index is (a,b,c) but b has a length, we can only pass the columns (a,b)
There was a problem hiding this comment.
Every specified length is used. So we should pass them.
model/model.go
Outdated
| @@ -135,6 +135,18 @@ func (t *TableInfo) GetPkName() CIStr { | |||
| return CIStr{} | |||
| } | |||
|
|
|||
| // GetPkColInfo gets the ColumnInfo of pk if exists. | |||
| func (t *TableInfo) GetPkColInfo() *ColumnInfo { | |||
| if t.PKIsHandle { | |||
util/ranger/new_refiner.go
Outdated
| } | ||
|
|
||
| // RangeSlice2IntRangeSlice changes []types.Range to []types.IntColumnRange | ||
| func RangeSlice2IntRangeSlice(ranges []types.Range) []types.IntColumnRange { |
There was a problem hiding this comment.
This function's name is too long. I think Ranges2IntRanges is ok. The following are same.
|
LGTM |
|
@shenli PTAL |
|
@shenli PTAL |
model/model.go
Outdated
| // GetPkColInfo gets the ColumnInfo of pk if exists. | ||
| func (t *TableInfo) GetPkColInfo() *ColumnInfo { | ||
| for _, colInfo := range t.Columns { | ||
| if mysql.HasPriKeyFlag(colInfo.Flag) { |
There was a problem hiding this comment.
composed pk currently is treated as a index.
There was a problem hiding this comment.
For all the columns in a composed pk, they will have the PriKeyFlag. Will this miss lead you?
There was a problem hiding this comment.
TableInfo.PkIsHandle checked outside when call this method. Move it in here?
There was a problem hiding this comment.
If the logic is right, you can keep it untouched. But please add a comment here.
expression/column.go
Outdated
| return nil | ||
| } | ||
|
|
||
| // IndexInfo2Cols changes a indexInfo's []*IndexColumn to corresponding []*Column and the length of each *IndexColumn. |
There was a problem hiding this comment.
The first sentence is not clear.
|
LGTM |
This method use []*expression.Column as parameter and both IntRange, ColumnRange, IndexRange can use this method.
@hanfei1991 PTAL